Skip to content

Lint Check for Raw Directives #930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jul 15, 2025
Merged

Lint Check for Raw Directives #930

merged 5 commits into from
Jul 15, 2025

Conversation

Malmahrouqi3
Copy link
Collaborator

@Malmahrouqi3 Malmahrouqi3 commented Jul 8, 2025 •
edited by qodo-merge-pro bot
Loading

User description

Description

Concerning(#928),
Quick check for old raw directives.

PR Type

Enhancement

Description

  • Add CI check for raw !$acc or !$omp directives

  • Enforce macro paradigm usage in source code

  • Exclude specific macro files from directive check

Changes diagram

flowchart LR
  A["Source Code"] --> B["CI Lint Check"]
  B --> C["Scan for Raw Directives"]
  C --> D["Exclude Macro Files"]
  D --> E["Fail if Found"]
Loading

Changes walkthrough 📝

Relevant files
Continuous-integration
lint-source.yml
Add raw directive detection to CI                                               

.github/workflows/lint-source.yml

  • Add new CI step to check for raw !$acc or !$omp directives
  • Use grep with exclusions for parallel_macros.fpp and syscheck.fpp
  • Fail build if any raw directives are found in source code
    		• +4/-0     
    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
    •

    @Copilot Copilot AI review requested due to automatic review settings July 8, 2025 17:21
    @Malmahrouqi3 Malmahrouqi3 requested a review from sbryngelson as a code owner July 8, 2025 17:21
    @qodo-merge-pro Qodo Merge Pro
    Copy link

    qodo-merge-pro bot commented Jul 8, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    928 - Fully compliant

    Compliant requirements:

    • Add CI check for raw !$acc or !$omp directives
    • Fail if any raw directives are found in source code
    • Exclude macro.fpp type files from the check
    • Enforce macro paradigm usage instead of raw directives

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Regex Pattern

    The grep pattern uses !\$acc\|!\$omp which may not match all variations of OpenMP/OpenACC directives. Consider if patterns like !$ACC, !$OMP, or with different spacing should also be caught.

     
    ! grep -iR '!\$acc\|!\$omp' --exclude="parallel_macros.fpp" --exclude="syscheck.fpp" ./src/*
    File Exclusions

    Only excludes parallel_macros.fpp and syscheck.fpp but ticket mentions excluding macro.fpp type files. Verify if other macro files should be excluded or if the current exclusions are sufficient.

     
    ! grep -iR '!\$acc\|!\$omp' --exclude="parallel_macros.fpp" --exclude="syscheck.fpp" ./src/*

    Copilot
    Copilot AI reviewed Jul 8, 2025
    View reviewed changes
    Copy link
    Contributor

    @Copilot Copilot AI left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Pull Request Overview

    This PR adds a new CI lint step to catch any raw OpenACC/OpenMP directives left in the source tree.

    • Introduce a GitHub Actions step named “Looking for raw directives” that greps for !$acc or !$omp in ./src
    • Exclude known macro files (parallel_macros.fpp, syscheck.fpp)
    Comments suppressed due to low confidence (2)

    .github/workflows/lint-source.yml:39

    • [nitpick] Limit the scan to Fortran source extensions to avoid false positives in non-code files. For example, add --include="*.f90" --include="*.fpp" after the -iR flag.
            ! grep -iR '!\$acc\|!\$omp' --exclude="parallel_macros.fpp" --exclude="syscheck.fpp" ./src/*

    .github/workflows/lint-source.yml:39

    • [nitpick] Exclude binary files to prevent grep errors on non-text files by adding -I (ignore binary) to the command, e.g., grep -iR -I.
            ! grep -iR '!\$acc\|!\$omp' --exclude="parallel_macros.fpp" --exclude="syscheck.fpp" ./src/*

    @qodo-merge-pro Qodo Merge Pro
    Copy link

    qodo-merge-pro bot commented Jul 8, 2025 •
    edited
    Loading

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @sbryngelson sbryngelson merged commit 6cb2376 into MFlowCode:master Jul 15, 2025
    18 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    Copilot code review Copilot Copilot left review comments

    @sbryngelson sbryngelson Awaiting requested review from sbryngelson sbryngelson is a code owner

    Assignees
    No one assigned
    Labels
    Review effort 2/5
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.
    2 participants
    @Malmahrouqi3 @sbryngelson